Skip to content

fix(tui): compact transcript retention without losing resize history#5239

Merged
can1357 merged 3 commits into
can1357:mainfrom
RensTillmann:fix/issue-4820-replay-safe-retention
Jul 14, 2026
Merged

fix(tui): compact transcript retention without losing resize history#5239
can1357 merged 3 commits into
can1357:mainfrom
RensTillmann:fix/issue-4820-replay-safe-retention

Conversation

@RensTillmann

Copy link
Copy Markdown
Contributor

Summary

  • compact finalized transcript head rows after they enter native scrollback, releasing retained render arrays in long sessions
  • rehydrate compacted history before destructive resize/reset full paints through a new NativeScrollbackReplay contract
  • keep versioned/post-finalize-mutable blocks out of compaction and preserve committed-block bookkeeping
  • bound the Markdown L2 render cache to 512 KiB total and 32 KiB per entry

Fixes #4820.

Context

This builds on the approach in #4841 by @cexll. That PR's review identified a P1 data-loss case: after compaction, a non-multiplexer resize or reset clears native scrollback but the compacted prefix cannot be replayed. This version adds an explicit pre-full-paint replay hook, so TranscriptContainer re-renders the complete child history for that paint and resumes compaction on the following frame.

Tests

  • bun test packages/tui/test/component-render.test.ts packages/coding-agent/test/modes/components/transcript-container.test.ts packages/tui/test/markdown.test.ts — 170 passed
  • bun run check:types in packages/tui — passed
  • bun run check:types in packages/coding-agent — passed
  • Biome check on all touched source/test files — passed
  • standalone binary build, --version, --help, and omp read /etc/hostname smoke tests — passed

Regression coverage

  • committed finalized head rows are dropped from subsequent frames
  • compacted rows remain classified as committed
  • destructive full paints call the replay hook before composition
  • replay survives the engine feeding its previous committed-row count before render
  • oversized Markdown renders are excluded from the shared L2 cache

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@tcf909

tcf909 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Follow-up fix for a live crash exposed by this PR's compaction path:

TypeError: undefined is not an object (evaluating 'segment.component') in TranscriptContainer.isBlockUncommitted when #retireIrcCard walks sparse compacted #segments holes after a later render.

PR: #5298

  • Guards undefined segment slots
  • Red/green regression for the sparse-hole path

@tcf909

tcf909 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Crash follow-up for this PR

Live crash after applying this PR:

TypeError: undefined is not an object (evaluating 'segment.component')
  at isBlockUncommitted
  at #retireIrcCard

Cause

#compactCommittedPrefix leaves zero-row placeholders, then a later render() allocates new Array(count) and only fills from #compactedChildStart, so compacted indices become sparse undefined holes. isBlockUncommitted() iterated every segment and crashed.

Fix

Guard undefined segment slots + red/green regression.

Verification

  • Red without guard: test fails with original TypeError
  • Green with guard: bun test packages/coding-agent/test/modes/components/transcript-container.test.ts → 48 pass / 0 fail
  • Focused install-tree suite for empty-stop + mixed-assistant + container: 59 pass / 0 fail

I opened #5298 but it was auto-closed (author not vouched). Please cherry-pick this onto the PR branch:

From c14513a8b03c26d989c78da68484302be926d7df Mon Sep 17 00:00:00 2001
From: "T.C. Ferguson" <tc.ferguson@forwardthinking.com>
Date: Sun, 12 Jul 2026 09:52:39 -1000
Subject: [PATCH] fix(tui): skip sparse compacted segments in
 isBlockUncommitted

After #5239 compaction, a later render only fills segments from
#compactedChildStart, leaving undefined holes in the prefix. Iterating
those entries crashed when retiring IRC/ephemeral cards
(`segment2.component`). Guard undefined segment slots and cover the
sparse-hole path with a red/green regression.
---
 .../modes/components/transcript-container.ts  |  6 +++++-
 .../components/transcript-container.test.ts   | 20 +++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/packages/coding-agent/src/modes/components/transcript-container.ts b/packages/coding-agent/src/modes/components/transcript-container.ts
index 428b46831..576dec064 100644
--- a/packages/coding-agent/src/modes/components/transcript-container.ts
+++ b/packages/coding-agent/src/modes/components/transcript-container.ts
@@ -242,9 +242,13 @@ export class TranscriptContainer
 	 */
 	isBlockUncommitted(component: Component): boolean {
 		const index = this.children.indexOf(component);
+		// Compacted prefix is already committed native history and must not be
+		// retracted. Compacted slots may be sparse holes after a later re-render
+		// (render only fills from #compactedChildStart), so the loop below must
+		// skip undefined entries.
 		if (index >= 0 && index < this.#compactedChildStart) return false;
 		for (const segment of this.#segments) {
-			if (segment.component !== component) continue;
+			if (segment === undefined || segment.component !== component) continue;
 			return segment.rowCount === 0 || segment.startRow >= this.#committedRows;
 		}
 		return true;
diff --git a/packages/coding-agent/test/modes/components/transcript-container.test.ts b/packages/coding-agent/test/modes/components/transcript-container.test.ts
index 56b24164c..6a812f0ba 100644
--- a/packages/coding-agent/test/modes/components/transcript-container.test.ts
+++ b/packages/coding-agent/test/modes/components/transcript-container.test.ts
@@ -700,6 +700,26 @@ describe("TranscriptContainer isBlockUncommitted", () => {
 		container.setNativeScrollbackCommittedRows(100);
 		expect(container.isBlockUncommitted(empty)).toBe(true);
 	});
+
+	it("survives sparse compacted segment holes when checking uncommitted status", () => {
+		const container = new TranscriptContainer();
+		const committed = new StreamingBlock(["committed"], true);
+		const live = new StreamingBlock(["live"], true);
+		container.addChild(committed);
+		container.addChild(live);
+
+		expect(container.render(40)).toEqual(["committed", "", "live"]);
+		container.setNativeScrollbackCommittedRows(2);
+		// Compaction first rewrites the compacted prefix into zero-row placeholders.
+		expect(container.render(40)).toEqual(["live"]);
+		// The next render only repopulates from #compactedChildStart, leaving a
+		// sparse hole at the compacted index. Retiring IRC/ephemeral cards walks
+		// every segment and must not crash on those undefined entries.
+		expect(container.render(40)).toEqual(["live"]);
+		expect(() => container.isBlockUncommitted(live)).not.toThrow();
+		expect(() => container.isBlockUncommitted(committed)).not.toThrow();
+		expect(container.isBlockUncommitted(committed)).toBe(false);
+	});
 });
 
 describe("TranscriptContainer renderViewportTail", () => {
-- 
2.50.1 (Apple Git-155)

tcf909 and others added 2 commits July 12, 2026 21:09
After can1357#5239 compaction, a later render only fills segments from
#compactedChildStart, leaving undefined holes in the prefix. Iterating
those entries crashed when retiring IRC/ephemeral cards
(`segment2.component`). Guard undefined segment slots and cover the
sparse-hole path with a red/green regression.
@can1357 can1357 merged commit 353bb5c into can1357:main Jul 14, 2026
19 checks passed
@roboomp roboomp added fix review:p3 triaged tui Terminal UI rendering and display ux User experience improvements labels Jul 14, 2026

@roboomp roboomp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: this closed PR is superseded by #5435, which retires child trees and rebases the TUI's aligned frame ledgers.
Blocking here: compacted children still retain their per-component rendered arrays, and the next post-compaction render leaves sparse #segments slots that crash isBlockUncommitted().
The advertised focused suites pass (170/170), but a direct two-block reproduction triggers the segment.component TypeError.
Thanks for the focused fix and replay-safety work, @RensTillmann.

Comment on lines +183 to +184
// Leading children whose rows were handed to native scrollback and dropped
// from the local frame. Children remain owned by the session and can be

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: Keeping every compacted child reachable leaves the per-component render arrays alive. For example, each retained Markdown descendant owns #cachedLines (packages/tui/src/components/markdown.ts:964-970); once this container starts its render walk at #compactedChildStart, those historical descendants are skipped but their caches remain referenced indefinitely. This trims only the aggregate #lines/segment references, not the one rendered array per historical block, so the claimed live-memory bound is not achieved. Please retire/dispose compacted child trees while rebasing the TUI's aligned ledgers, rather than retaining them solely for replay, or otherwise release every descendant render cache.

Comment on lines 243 to +245
isBlockUncommitted(component: Component): boolean {
const index = this.children.indexOf(component);
// Compacted prefix is already committed native history and must not be

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: A later post-compaction render allocates new Array(count) and fills only indices at/after #compactedChildStart, leaving sparse undefined slots in #segments. The for (const segment of this.#segments) immediately below then dereferences segment.component and crashes. I reproduced this by rendering two finalized blocks, committing the first two rows, rendering twice, then calling isBlockUncommitted(tail): TypeError: undefined is not an object (evaluating 'segment.component'). This is reachable from ephemeral-card retirement. Please skip undefined slots and add a regression that performs the second render before querying either block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix review:p3 triaged tui Terminal UI rendering and display ux User experience improvements vouched Passed the vouch gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Long resumed TUI sessions retain excessive JSC/WebKit footprint and block event loop during transcript/render work

4 participants